fix: clear key mismatch warning after successful purge resolution (#2349)#2365
Merged
fix: clear key mismatch warning after successful purge resolution (#2349)#2365
Conversation
#2349) After a successful purge + NodeInfo exchange, the stored key gets updated to match the mesh-received key. But the clearing block only fired when oldKey !== newKey, so the keyMismatchDetected flag persisted forever when keys already matched (post-purge scenario). Now clears the flag in both cases: when a new key arrives AND when keys already match (indicating the mismatch was resolved via device re-sync). Closes #2349 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
|
|
||
| // Existing block — only runs for PKI-error-based mismatches, NOT our proactive detection | ||
| // Clear mismatch flag when keys now match (post-purge resolution) |
There was a problem hiding this comment.
👍 Excellent improvement to the comment clarity. This clearly explains both resolution scenarios that the code now handles.
Comment on lines
4368
to
4375
| if (oldKey !== newKey) { | ||
| // Key has changed - the mismatch is fixed! | ||
| // Key has changed - the mismatch is fixed via new key | ||
| logger.info(`🔐 Key mismatch RESOLVED for node ${nodeId} (${user.longName}) - received new key`); | ||
| nodeData.keyMismatchDetected = false; | ||
| nodeData.lastMeshReceivedKey = undefined; | ||
| // Don't clear keySecurityIssueDetails if there's a low-entropy issue | ||
| if (!isLowEntropy) { | ||
| nodeData.keySecurityIssueDetails = undefined; | ||
| } | ||
|
|
||
| // Clear the repair state and log success | ||
| databaseService.clearKeyRepairStateAsync(fromNum); | ||
| const nodeName = user.longName || user.shortName || nodeId; | ||
| databaseService.logKeyRepairAttemptAsync(fromNum, nodeName, 'fixed', true); | ||
| } else { | ||
| // Keys now match - the mismatch was fixed (e.g., device re-synced after purge) | ||
| logger.info(`🔐 Key mismatch RESOLVED for node ${nodeId} (${user.longName}) - keys now match`); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Perfect fix for the core issue
This is the critical change that resolves #2349. The previous logic had a gap:
Before: Only cleared keyMismatchDetected when oldKey !== newKey (new key scenario)
After: Clears the flag for both scenarios - when a new key arrives AND when keys already match
The else branch handling oldKey === newKey is exactly what was missing. After a successful immediate purge + NodeInfo exchange, the device re-syncs and the keys match, but the old logic would skip clearing the flag entirely.
The solution correctly handles both resolution paths while maintaining appropriate logging for each case.
Comment on lines
+4376
to
4393
| nodeData.keyMismatchDetected = false; | ||
| nodeData.lastMeshReceivedKey = undefined; | ||
| // Don't clear keySecurityIssueDetails if there's a low-entropy issue | ||
| if (!isLowEntropy) { | ||
| nodeData.keySecurityIssueDetails = undefined; | ||
| } | ||
|
|
||
| // Clear the repair state and log success | ||
| databaseService.clearKeyRepairStateAsync(fromNum); | ||
| const nodeName = user.longName || user.shortName || nodeId; | ||
| databaseService.logKeyRepairAttemptAsync(fromNum, nodeName, 'fixed', true); | ||
|
|
||
| // Emit update to UI | ||
| dataEventEmitter.emitNodeUpdate(fromNum, { | ||
| keyMismatchDetected: false, | ||
| keySecurityIssueDetails: isLowEntropy ? nodeData.keySecurityIssueDetails : undefined | ||
| }); | ||
| } |
There was a problem hiding this comment.
✅ Good refactoring: Moving the common cleanup code outside the conditional ensures it runs for both resolution scenarios. This eliminates the duplication that would have been needed and ensures consistent state cleanup regardless of how the mismatch was resolved.
Yeraze
added a commit
that referenced
this pull request
Mar 22, 2026
PKI_UNKNOWN_PUBKEY and PKI_SEND_FAIL_PUBLIC_KEY routing errors were suppressed when the target node wasn't in the radio's device database. The guard assumed this was only expected after factory reset/purge, but it also suppressed legitimate key mismatch detection for nodes the radio simply doesn't know about. Removed the isNodeInDeviceDb() guard from both routing error paths (request packets and message-tracked packets). The key mismatch flag will now clear naturally via the resolution logic in PR #2365 when keys are re-synced. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 task
Yeraze
added a commit
that referenced
this pull request
Mar 22, 2026
* fix: PKI_UNKNOWN_PUBKEY errors now correctly flag key mismatch PKI_UNKNOWN_PUBKEY and PKI_SEND_FAIL_PUBLIC_KEY routing errors were suppressed when the target node wasn't in the radio's device database. The guard assumed this was only expected after factory reset/purge, but it also suppressed legitimate key mismatch detection for nodes the radio simply doesn't know about. Removed the isNodeInDeviceDb() guard from both routing error paths (request packets and message-tracked packets). The key mismatch flag will now clear naturally via the resolution logic in PR #2365 when keys are re-synced. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test+docs: add PKI error detection tests and architecture documentation - 9 new tests for isPkiError, RoutingError constants, getPortNumName - Document PKI error detection rules in ARCHITECTURE_LESSONS.md - Anti-pattern: never gate PKI detection on isNodeInDeviceDb() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2349 — the key mismatch warning persisted indefinitely even after a successful key exchange.
Root Cause
After a successful immediate purge + NodeInfo exchange:
existingNode.publicKey === nodeData.publicKey(keys match), so the mismatch detection block at line 4311 is skippedoldKey !== newKeyis false (keys already match), so it never clears the flagkeyMismatchDetectedstaystrueforever in the databaseFix
Remove the
oldKey !== newKeygate on the clearing block. IfkeyMismatchDetectedis true and we're not seeing a new mismatch, clear it regardless — keys matching means the mismatch is resolved.Files Changed
src/server/meshtasticManager.tskeyMismatchDetectedwhen keys match (post-purge) in addition to when a new key arrivesTest plan
🤖 Generated with Claude Code